fix(declarative): allow bundled custom components without AIRBYTE_ENABLE_UNSAFE_CODE - #1083
Conversation
…BLE_UNSAFE_CODE Only gate Custom* components when the manifest itself is supplied through the config, which is the untrusted case. Manifests bundled in a published connector image are trusted, so their custom components must keep working in environments that do not set AIRBYTE_ENABLE_UNSAFE_CODE, such as Airbyte Cloud. Co-Authored-By: syed.khadeer@airbyte.io <cloud-support@airbyte.io>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksTesting This CDK VersionYou can test this version of the CDK using the following: # Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/airbyte-python-cdk.git@devin/1785220224-fix-bundled-custom-components#egg=airbyte-python-cdk[dev]' --help
# Update a connector to use the CDK from this branch ref:
cd airbyte-integrations/connectors/source-example
poe use-cdk-branch devin/1785220224-fix-bundled-custom-componentsPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
…_components_trusted Co-Authored-By: syed.khadeer@airbyte.io <cloud-support@airbyte.io>
Runtime verificationTested by recreating the published The reported breakage is reproduced on
|
main (7e96546) |
this branch | |
|---|---|---|
spec |
✅ (proves nothing) | ✅ |
discover |
❌ AirbyteCustomCodeNotPermittedError, exit 1, no catalog |
✅ exit 0, 27 streams (customer, invoice, subscription, item, item_price, plan, coupon, …) |
check |
❌ FAILED: Encountered an error while discovering streams. Error: Custom connector code is not permitted… |
✅ reaches Chargebee, FAILED: HTTP 401 Unauthorized (fake API key — no live creds) |
The component is genuinely instantiated, not silently skipped:
customer…record_selector.transformations -> source_declarative_manifest.components.CustomFieldTransformation
and a full mocked read of customer folds custom fields correctly:
{"id":"cust_1","email":"a@b.c","updated_at":1700000000,
"custom_fields":[{"name":"cf_color","value":"blue"}]} # cf_color removed
Security gate: still holds on every caller-supplied route
A poison module writes /tmp/PWNED at import time; it is deleted before each case, so its presence proves arbitrary code actually ran.
| Route (env var unset) | Gate fired | Code executed |
|---|---|---|
SDM --manifest-path |
✅ | no |
SDM config w/ __injected_declarative_manifest |
✅ | no |
connector_builder.main read (test_read) |
✅ | no |
$ref-nested Custom* in injected manifest |
✅ | no |
__injected_components_py |
✅ | no |
control: AIRBYTE_ENABLE_UNSAFE_CODE=true |
n/a | yes (catalog built — test not vacuous) |
An earlier revision of this PR keyed the gate solely off config[INJECTED_MANIFEST], which left one route ungated: manifest_server passes the caller's manifest as source_config outside the config, so build_source built streams and imported the poisoned class_name with no exception. That is why the second commit adds the explicit custom_components_trusted=False signal at that call site. Note manifest_server/routers/capabilities.py:23 still advertises custom_code_execution from AIRBYTE_ENABLE_UNSAFE_CODE, which now matches the enforced behavior again.
Regression / quality gates
pytest unit_tests/sources/declarative unit_tests/legacy/sources/declarative
=> 1736 passed
pytest unit_tests/sources/declarative/parsers/test_model_to_component_factory.py \
unit_tests/manifest_server unit_tests/connector_builder
=> 233 passed
ruff check . => All checks passed!
ruff format --check . => all files already formatted
mypy airbyte_cdk => Success: no issues found in 454 source files
Not tested: live Chargebee check (no credentials available); the published Docker image was not rebuilt — the image layout was reproduced locally from the CDK Dockerfile.
📝 WalkthroughWalkthrough
ChangesCustom component trust
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ManifestCaller
participant ManifestEntryPoint
participant ConcurrentDeclarativeSource
participant ModelToComponentFactory
participant CustomCodeGate
ManifestCaller->>ManifestEntryPoint: provide runtime manifest
ManifestEntryPoint->>ConcurrentDeclarativeSource: set custom_components_trusted=false
ConcurrentDeclarativeSource->>ModelToComponentFactory: forward trust setting
ModelToComponentFactory->>CustomCodeGate: evaluate custom component
CustomCodeGate-->>ModelToComponentFactory: permit bundled or reject untrusted component
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
/prerelease
|
…rusted Co-Authored-By: Daryna Ishchenko <darina.ishchenko17@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@airbyte_cdk/cli/source_declarative_manifest/_run.py`:
- Around line 215-218: Guard the `_register_components_from_file` call in the
manifest execution flow with the `AIRBYTE_ENABLE_UNSAFE_CODE` check before
loading or executing caller-supplied components. Ensure `--components-path` is
rejected or skipped when unsafe code is disabled, so `spec.loader.exec_module`
cannot run before the `custom_components_trusted=False` protection is applied.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 59d88db1-5dd1-4651-afdc-6c38a541756d
📒 Files selected for processing (3)
airbyte_cdk/cli/source_declarative_manifest/_run.pyairbyte_cdk/connector_builder/connector_builder_handler.pyunit_tests/connector_builder/test_connector_builder_handler.py
…FE_CODE Co-Authored-By: Daryna Ishchenko <darina.ishchenko17@gmail.com>
|
/prerelease
|
6ce33ea
into
main
3 participants
Summary
Fixes #1082: since 7.23.7, every published manifest-only connector that bundles a
components.pyfailscheck/discover/readin Airbyte Cloud withAirbyteCustomCodeNotPermittedError. Found while triaging a production Chargebee -> BigQuery connection that broke the momentsource-chargebee0.10.41 (base imagesource-declarative-manifest:7.23.7) rolled out.#1076 added the
AIRBYTE_ENABLE_UNSAFE_CODEgate tocreate_custom_componentunconditionally, which lost the provenance distinction the gate has always had: it is meant to protect against untrusted code arriving from a caller (__injected_components_py/ a caller-supplied manifest from Connector Builder), not against a connector's own code baked into its own image. Cloud never setsAIRBYTE_ENABLE_UNSAFE_CODE, so trusted first-party connectors started failing.The gate now keys off manifest provenance instead of the environment alone:
Two provenance signals, because untrusted manifests reach the factory two different ways:
__injected_declarative_manifestin the config.custom_components_trusted=False, a new kwarg threadedConcurrentDeclarativeSource->ModelToComponentFactory(and on tosubstream_factory).A manifest read from
/airbyte/integration_code/source_declarative_manifest/manifest.yamlinside a published image hits neither signal and may use its bundled components, as before 7.23.7. #1076's threat model (a manifest pointingclass_nameat any importable callable, e.g.os.getcwd) stays blocked everywhere it applied.Security follow-up:
custom_components_trusted=Falseon every caller-supplied entry pointReview of the first cut found that the
config.get(INJECTED_MANIFEST)signal alone is bypassable, so relying on it at the Connector Builder /source-declarative-manifestremote entry points reopened the #1076 RCE (GHSA-g35w-6r6v-h4r7). Two ways, both verified:ConcurrentDeclarativeSource.__init__builds the spec component with an empty dict config (create_component(SpecModel, spec, dict())), so aCustomConfigTransformation/CustomValidationStrategyinspec.config_normalization_rulesreachescreate_custom_componentwith noINJECTED_MANIFESTkey.ConfigRemoveFieldson["__injected_declarative_manifest"]runs during config normalization before streams are built, so laterCustom*components see a config with the marker gone.Both routes are the exact attack surface #1076 closed. Because
manifest_server.build_sourcewas the only caller passing the trust flag, the other two untrusted entry points were exposed. Fix: passcustom_components_trusted=Falsein both:connector_builder/connector_builder_handler.py::create_sourcecli/source_declarative_manifest/_run.py::create_declarative_source(remote-manifest path only; the bundled published connector uses the local-manifest path, which stays trusted)The trust flag lives on the factory, not in the config, so it is immune to both bypasses.
config.get(INJECTED_MANIFEST)is now redundant defense-in-depth rather than the primary control.Blast radius of the regression
56 monorepo connectors bundle a
components.py. Per the live registry, 19 are published on 7.23.7 and broken in Cloud today — apify-dataset, bing-ads, chargebee, google-search-console, greenhouse, instagram, klaviyo, linkedin-ads, monday, nexus-datasets, notion, outreach, paypal-transaction, pinterest, pylon, qualaroo, recurly, retently, slack (full table in #1082 (comment)) — with 13 more on 7.23.6 that would break at their next base-image bump. All 19 need a rebuild once this ships.Tests
AIRBYTE_ENABLE_UNSAFE_CODEto instantiateCustom*components from YAML #1076's gate test now injects a manifest into the config and is parametrized over a bundled component class andos.getcwd, asserting the gate fires beforeclass_nameis resolved;test_create_custom_component_permitted_for_bundled_manifestcovers the image-bundled case with the env var unset;test_create_custom_component_requires_custom_code_enabled_when_untrustedcovers the manifest-server flag. The two autouseconftest.pyfixtures fix(declarative): requireAIRBYTE_ENABLE_UNSAFE_CODEto instantiateCustom*components from YAML #1076 added to force the env var on across the declarative test suites are removed — they masked exactly this regression.create_sourceentry point withAIRBYTE_ENABLE_UNSAFE_CODEunset:test_create_source_marks_manifest_untrusted(invariant),test_spec_level_custom_component_is_gated,test_config_strip_does_not_bypass_custom_code_gate. All three fail on the pre-follow-up branch and pass with the fix.source-declarative-manifestimage layout with source-chargebee 0.10.41's realmanifest.yaml+components.pyand running the connector CLI withAIRBYTE_ENABLE_UNSAFE_CODEunset:discoverfails onmainand returns 27 streams here,CustomFieldTransformationis genuinely instantiated, and a poisonedclass_nameis still blocked on every caller-supplied route.source-declarative-manifest:7.23.7) using real integration-test credentials, bundledmanifest.yaml+components.py, andAIRBYTE_ENABLE_UNSAFE_CODEunset:checkfails withAirbyteCustomCodeNotPermittedErroron released CDK 7.23.7 and succeeds on this branch.--components-pathgatingThe remaining pre-existing gap flagged in review:
_register_components_from_fileexecuted a caller-supplied--components-pathfile viaexec_modulebefore any gate could fire, so the trust flag offered no protection on that CLI route. It is now gated: the helper checkscustom_code_execution_permitted()and raisesAirbyteCustomCodeNotPermittedErrorbefore the module is created or executed. Local-operator workflows using--components-pathmust setAIRBYTE_ENABLE_UNSAFE_CODE=true(same requirement as before #1076's regression for injected code). Covered bytest_register_components_from_file_is_gated.Summary by CodeRabbit
Link to Devin session: https://app.devin.ai/sessions/20f99bb504b543fda97972948b0ceecb
Requested by: Daryna Ishchenko (@darynaishchenko)